Why?

N-API is the latest interface for native addons in node.js. It's a set of c functions that lie between your native addon and v8. Main benefit is that your addon is independent of the v8 version, which means it will be compatible with future versions of node.js.

So, as long as your native code doesn't change, it doesn't need to be recompiled as new versions of node.js are released. This helps greatly with both maintenance and also when delivering code to the users of your module.

The downside is a loss of performance since you're no longer interfacing directly with v8.

Porting to N-API

There's been some work done already to port leveldown to N-API but I decided to give it a go myself. Mainly because accepting a huge PR as a maintainer means a lot of work, it's not much more work to actually do it yourself.

Also, personally, I've always felt that the v8 "lingo" and abstractions were difficult to understand. Basically, you need to understand how v8 works (to some extent) in order to write good native addons. Maybe it was just over my head.

Design

However, I think "workers" as a concept is easy to understand, so I've tried to keep the code similar to the nan approach in this sense.

But why all these C++ classes/structs? Why not use c structs? They are simpler!

LevelDB is a C++ library and the glue code was already written in C++. Also, I personally like virtual methods, which really come in handy when creating workers. You can abuse virtual methods as well, but if you keep it to a minimum (no multiple inheritance) it can be quite elegant.

Another reason for this was for simplicity during rewrite. The main focus when migrating from nan to N-API should be at the intersection of js and c++ land. The code in between should really not have to change that much.

Why one binding.cc? The file has so many lines!

Correct. At first I wanted to keep a single file to make it easier during rewrite, but the more I thought of it, the more it made sense to keep everything in one amalgamated file. I'd prefer to keep it this way. It's so much easier to find things. Each async method has its worker right next to it.

Feedback

How can we help you review this?

Since this PR is quite big, I'd prefer to keep review at a bigger level. Please let me know if there's something in the code that's a big no no, e.g. if I'm using the wrong abstractions.

Input such as "Did you know you can do X instead of Y and then you don't have to Z?" is also very helpful.

Another important thing is that when you rewrite things and there are many details to keep in your head, you often forget things. So if you can find something that I have forgotten, that'd be great.

One thing that would be greatly needed is also help with finding bugs. It would be really helpful if you can make the code segfault. Also running benchmarks to compare with the previous implementation would be nice.

Some stats

I tried running test-for -n 100 for pure tests (e.g. only the tape tests, without standard etc) and got the same average for both the master and the napi branch (~3.8s/test). Timing tests is most likely not a good measure, but it's a indication at least.

Total lines of native code in src/:

$ wc -l src/*
   32 src/async.h
   17 src/batch_async.cc
   26 src/batch_async.h
  148 src/batch.cc
   40 src/batch.h
   33 src/common.h
  290 src/database_async.cc
  168 src/database_async.h
  514 src/database.cc
  108 src/database.h
   86 src/iterator_async.cc
   43 src/iterator_async.h
  613 src/iterator.cc
   94 src/iterator.h
   38 src/leveldown_async.cc
   34 src/leveldown_async.h
   70 src/leveldown.cc
  117 src/leveldown.h
 2471 total

Compared to binding.cc:

$ wc -l binding.cc
1772